Cleaned up public app release scripts#29414
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughPublic app automation now uses ESM entrypoints and shared metadata utilities. CDN purge matrices derive URLs from Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run ghost:test:integration |
✅ Succeeded | 2m 59s | View ↗ |
nx run ghost:test:ci:integration |
✅ Succeeded | 4s | View ↗ |
nx run @tryghost/admin:test:acceptance |
✅ Succeeded | 5m 27s | View ↗ |
nx run ghost:test:legacy |
✅ Succeeded | 2m 55s | View ↗ |
nx run ghost:test:e2e |
✅ Succeeded | 2m 39s | View ↗ |
nx run @tryghost/koenig-lexical:test:acceptance |
✅ Succeeded | 2m 28s | View ↗ |
nx run @tryghost/comments-ui:test:acceptance |
✅ Succeeded | 40s | View ↗ |
nx run @tryghost/activitypub:test:acceptance |
✅ Succeeded | 41s | View ↗ |
Additional runs (8) |
✅ Succeeded | ... | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-07-17 18:06:24 UTC
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/check-app-version-bump.js`:
- Line 171: Replace the import.meta.main guard in the script entrypoint with a
Node.js-version-compatible check so the app-version consistency validation
always runs when executed directly, including on versions before 21.2.0. Keep
the existing check logic unchanged and avoid relying on import.meta.main.
In `@scripts/release-apps.js`:
- Around line 77-83: Update updatePackageJson to pass 4 as the JSON.stringify
indentation argument when writing package.json, preserving the repository’s
existing formatting and avoiding unrelated reformatting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: beaafa74-da4e-43aa-b038-739eb01f93d4
📒 Files selected for processing (17)
.github/workflows/ci.ymlapps/admin-toolbar/package.jsonapps/announcement-bar/package.jsonapps/comments-ui/package.jsonapps/portal/package.jsonapps/signup-form/package.jsonapps/sodo-search/package.jsonscripts/README.mdscripts/build-public-apps-matrix.cjsscripts/build-public-apps-matrix.jsscripts/check-app-version-bump.jsscripts/lib/public-apps.jsscripts/public-apps.jsonscripts/release-apps.cjsscripts/release-apps.jsscripts/test/build-public-apps-matrix.test.jsscripts/test/public-apps.test.js
💤 Files with no reviewable changes (2)
- scripts/build-public-apps-matrix.cjs
- scripts/release-apps.cjs
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #29414 +/- ##
==========================================
- Coverage 74.32% 74.28% -0.04%
==========================================
Files 1597 1597
Lines 139677 139677
Branches 16963 16949 -14
==========================================
- Hits 103812 103763 -49
- Misses 34846 34863 +17
- Partials 1019 1051 +32
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
0b47d92 to
b8db993
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/check-app-version-bump.js (1)
154-161: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid redundant file reads by extracting variables.
Currently,
getPrVersionandgetPrDefaultsVersionare called twice per app (once for the consistency check and once for the console log). Each call synchronously reads and parsespackage.jsonordefaults.json. Storing the results in local variables improves readability and avoids redundant I/O operations.♻️ Proposed refactor
- const error = checkAppConsistency(app, getPrVersion(app), getPrDefaultsVersion(app)); + const prVersion = getPrVersion(app); + const prDefaultsVersion = getPrDefaultsVersion(app); + const error = checkAppConsistency(app, prVersion, prDefaultsVersion); if (error) { failedApps.push(error); continue; } - console.log(`${app.configKey} version consistency check passed (package.json ${majorMinor(getPrVersion(app))} = defaults.json ${getPrDefaultsVersion(app)})`); + console.log(`${app.configKey} version consistency check passed (package.json ${majorMinor(prVersion)} = defaults.json ${prDefaultsVersion})`);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check-app-version-bump.js` around lines 154 - 161, In the app consistency-check loop, update the flow around checkAppConsistency to store getPrVersion(app) and getPrDefaultsVersion(app) results in local variables, then reuse those variables for both the consistency check and success log. Preserve the existing error handling and log output.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scripts/check-app-version-bump.js`:
- Around line 154-161: In the app consistency-check loop, update the flow around
checkAppConsistency to store getPrVersion(app) and getPrDefaultsVersion(app)
results in local variables, then reuse those variables for both the consistency
check and success log. Preserve the existing error handling and log output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 61a8373f-ca53-4d78-8e5b-60a346aef5c2
📒 Files selected for processing (20)
.github/workflows/ci.ymlapps/admin-toolbar/package.jsonapps/announcement-bar/package.jsonapps/comments-ui/package.jsonapps/portal/package.jsonapps/signup-form/package.jsonapps/sodo-search/package.jsonscripts/README.mdscripts/build-public-apps-matrix.cjsscripts/build-public-apps-matrix.jsscripts/check-app-version-bump.jsscripts/lib/public-apps.jsscripts/lib/release-notes.jsscripts/lib/resolve-base-tag.jsscripts/lib/utils.jsscripts/public-apps.jsonscripts/release-apps.cjsscripts/release-apps.jsscripts/test/build-public-apps-matrix.test.jsscripts/test/public-apps.test.js
💤 Files with no reviewable changes (2)
- scripts/build-public-apps-matrix.cjs
- scripts/release-apps.cjs
🚧 Files skipped from review as they are similar to previous changes (10)
- apps/signup-form/package.json
- apps/sodo-search/package.json
- apps/comments-ui/package.json
- apps/portal/package.json
- scripts/build-public-apps-matrix.js
- scripts/test/public-apps.test.js
- apps/announcement-bar/package.json
- apps/admin-toolbar/package.json
- scripts/release-apps.js
- scripts/test/build-public-apps-matrix.test.js
b717d36 to
e198bae
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/lib/public-apps.js (1)
39-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider throwing an explicit error if the app is not found.
If an unrecognized package name is passed,
findwill returnundefined. This will cause downstream code (such as the release runner) to throw a genericTypeError(e.g.,Cannot read properties of undefined (reading 'configKey')) when it tries to access properties on the result. Throwing an explicit error here can improve debugging.♻️ Proposed refactor
-/** `@param` {string} packageName */ -export const appForPackageName = packageName => PUBLIC_APPS.find(app => app.packageName === packageName); +/** `@param` {string} packageName */ +export const appForPackageName = packageName => { + const app = PUBLIC_APPS.find(a => a.packageName === packageName); + if (!app) { + throw new Error(`App "${packageName}" not found in public-apps.json`); + } + return app; +};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/lib/public-apps.js` around lines 39 - 40, Update appForPackageName to detect when PUBLIC_APPS.find returns no match and throw a descriptive error that includes the unrecognized packageName; preserve returning the matched app unchanged for valid package names.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scripts/lib/public-apps.js`:
- Around line 39-40: Update appForPackageName to detect when PUBLIC_APPS.find
returns no match and throw a descriptive error that includes the unrecognized
packageName; preserve returning the matched app unchanged for valid package
names.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fcbc186d-674a-457a-a0a3-2134cb594fd6
📒 Files selected for processing (23)
.github/workflows/ci.ymlapps/admin-toolbar/package.jsonapps/announcement-bar/package.jsonapps/comments-ui/package.jsonapps/portal/package.jsonapps/signup-form/package.jsonapps/sodo-search/package.jsonscripts/README.mdscripts/build-public-apps-matrix.cjsscripts/build-public-apps-matrix.jsscripts/check-app-version-bump.jsscripts/lib/constants.jsscripts/lib/public-apps.jsscripts/lib/release-notes.jsscripts/lib/resolve-base-tag.jsscripts/lib/utils.jsscripts/public-apps.jsonscripts/release-apps.cjsscripts/release-apps.jsscripts/release.cjsscripts/test/build-public-apps-matrix.test.jsscripts/test/public-apps.test.jsscripts/test/resolve-base-tag.test.cjs
💤 Files with no reviewable changes (2)
- scripts/build-public-apps-matrix.cjs
- scripts/release-apps.cjs
🚧 Files skipped from review as they are similar to previous changes (13)
- apps/sodo-search/package.json
- apps/comments-ui/package.json
- apps/announcement-bar/package.json
- apps/portal/package.json
- scripts/test/resolve-base-tag.test.cjs
- apps/signup-form/package.json
- scripts/release.cjs
- scripts/public-apps.json
- scripts/build-public-apps-matrix.js
- scripts/test/public-apps.test.js
- scripts/README.md
- scripts/test/build-public-apps-matrix.test.js
- scripts/release-apps.js
e198bae to
64be978
Compare
ref https://linear.app/ghost/issue/PLA-235/consolidate-ci-scripts-into-monorepo-package - convert public app-related scripts to esm - simplify public-apps and infer cdn paths from defaults.json - dedupe code - remove unnecessary changelog generation for public app commits
64be978 to
7a68445
Compare
1 participant
ref https://linear.app/ghost/issue/PLA-235/consolidate-ci-scripts-into-monorepo-package